Skip to content

Edit alert flyout#58964

Merged
YulNaumenko merged 17 commits intoelastic:masterfrom
YulNaumenko:alerting_ui_edit_alert_flyout
Mar 4, 2020
Merged

Edit alert flyout#58964
YulNaumenko merged 17 commits intoelastic:masterfrom
YulNaumenko:alerting_ui_edit_alert_flyout

Conversation

@YulNaumenko
Copy link
Copy Markdown
Contributor

@YulNaumenko YulNaumenko commented Feb 29, 2020

Summary

Resolve #51545

Checklist

Delete any items that are not applicable to this PR.

@YulNaumenko YulNaumenko added Feature:Alerting v8.0.0 release_note:skip Skip the PR/issue when compiling release notes v7.7.0 Team:ResponseOps Platform ResponseOps team (formerly the Cases and Alerting teams) t// labels Feb 29, 2020
@YulNaumenko YulNaumenko requested a review from a team as a code owner February 29, 2020 01:29
@YulNaumenko YulNaumenko self-assigned this Feb 29, 2020
@elasticmachine
Copy link
Copy Markdown
Contributor

Pinging @elastic/kibana-alerting-services (Team:Alerting Services)

…alert_flyout

# Please enter a commit message to explain why this merge is necessary,
# especially if it merges an updated upstream into a topic branch.
#
# Lines starting with '#' will be ignored, and an empty message aborts
# the commit.
Copy link
Copy Markdown
Contributor

@gmmorris gmmorris left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't been able to get the UI to work for me yet, so not approving, but I can't see anything worrying, so happy to approve once I get to test the UI. :)

Comment on lines +78 to +81
// TODO: uncomment variables test when server API will be ready
// await testSubjects.click('slackAddVariableButton');
// const variableMenuButton = await testSubjects.find('variableMenuButton-0');
// await variableMenuButton.click();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Has this been forgotten or is it still in progress? :)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In progress, we have an issue for this #58529

…alert_flyout

# Conflicts:
#	x-pack/plugins/triggers_actions_ui/public/application/components/builtin_alert_types/threshold/expression.tsx
#	x-pack/test/functional_with_es_ssl/apps/triggers_actions_ui/alerts.ts
@YulNaumenko YulNaumenko requested a review from gmmorris March 3, 2020 17:40
…alert_flyout

# Please enter a commit message to explain why this merge is necessary,
# especially if it merges an updated upstream into a topic branch.
#
# Lines starting with '#' will be ignored, and an empty message aborts
# the commit.
…alert_flyout

# Conflicts:
#	x-pack/plugins/triggers_actions_ui/public/application/sections/alert_form/alert_add.test.tsx
Copy link
Copy Markdown
Contributor

@gmmorris gmmorris left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Talked with @YulNaumenko about a couple of points and a UI bug - all seems to already be known and documented in other issues, so approving this. 👍

Copy link
Copy Markdown
Contributor

@pmuellr pmuellr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

code LGTM

I loaded up the PR, was able to edit an alert \o/

A few issues; have to modify the interval number to save (but also on create), and an alert saved with an interval 1s comes up in the edit form as 1 minute instead. We can open these as separate issues. Let's ship this thing!

@kibanamachine
Copy link
Copy Markdown
Contributor

💚 Build Succeeded

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@YulNaumenko YulNaumenko merged commit daf6226 into elastic:master Mar 4, 2020
YulNaumenko added a commit to YulNaumenko/kibana that referenced this pull request Mar 4, 2020
* Implemented edit alert functionality

* Added unit tests

* Added functional test for edit alert

* Fixed failed tests

* Fixed edit api

* Fixed due to comments

* Fixed functional test

* Fixed tests

* Fixed add alert

* Small type fix

* Fixed jest test

* Fixed type check

* Fixed bugs with interval and throttle + index threshold expression
YulNaumenko added a commit that referenced this pull request Mar 5, 2020
* Edit alert flyout (#58964)

* Implemented edit alert functionality

* Added unit tests

* Added functional test for edit alert

* Fixed failed tests

* Fixed edit api

* Fixed due to comments

* Fixed functional test

* Fixed tests

* Fixed add alert

* Small type fix

* Fixed jest test

* Fixed type check

* Fixed bugs with interval and throttle + index threshold expression

* Removed before all from alert jest tests

* Fixed type check
jloleysens added a commit to jloleysens/kibana that referenced this pull request Mar 5, 2020
…re/files-and-filetree

* 'master' of github.com:elastic/kibana: (254 commits)
  Convert discover_page to ts, remove redundunt methods (elastic#59312)
  [Fix for Vis Editor] Revert setting time field to empty string when it's undefined (elastic#58873)
  Delete legacy search endpoint (elastic#59341)
  [Uptime] Improve duration chart (elastic#58404)
  [Snapshot & Restore] NP migration (elastic#59109)
  [ML] Add support for date_nanos time field in anomaly job wizard (elastic#59017)
  Revert "Makes alerting and actions optional properties for interface RequestH… (elastic#59264)"
  Change remote_clusters ID to remoteClusters (elastic#59246)
  Makes alerting and actions optional properties for interface RequestH… (elastic#59264)
  Clean up date histogram agg type. (elastic#58805)
  [ML] Management: fix license unsubscribe (elastic#59365)
  Remove documentation for server.cors settings (elastic#59096)
  Edit alert flyout (elastic#58964)
  [SIEM] Fix rule delete/duplicate actions (elastic#59306)
  move mouse to close obstructing tooltip (elastic#59214)
  Reset page after deleting (elastic#59310)
  Make sure phrases input filter triggers autosuggestons (elastic#59299)
  Add loading count source for http requests (elastic#59245)
  Revert "[ML] Transforms: Deprecate custom KibanaContext. (elastic#59133)"
  Expose metrics service to public API (elastic#59294)
  ...

# Conflicts:
#	src/plugins/console/public/application/containers/editor/legacy/console_editor/editor.tsx
spong pushed a commit that referenced this pull request Mar 24, 2020
## Summary

Allow defining notifications that will trigger whenever the rule created new signals.

Requires:
- #58395
- #58964
- #60832


![Screenshot 2020-03-02 at 10 19 18](https://user-images.githubusercontent.com/5188868/75662390-4fe8bf00-5c6f-11ea-943f-591367348b91.png)

![Screenshot 2020-03-02 at 10 13 00](https://user-images.githubusercontent.com/5188868/75662421-5e36db00-5c6f-11ea-9317-d158cddf4344.png)


### Checklist

Delete any items that are not applicable to this PR.

- [ ] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/master/packages/kbn-i18n/README.md)
- [ ] [Documentation](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#writing-documentation) was added for features that require explanation or tutorials
- [ ] [Unit or functional tests](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#cross-browser-compatibility) were updated or added to match the most common scenarios
- [ ] This was checked for [keyboard-only and screenreader accessibility](https://developer.mozilla.org/en-US/docs/Learn/Tools_and_testing/Cross_browser_testing/Accessibility#Accessibility_testing_checklist)
- [ ] This renders correctly on smaller devices using a responsive layout. (You can test this [in your browser](https://www.browserstack.com/guide/responsive-testing-on-local-server)
- [ ] This was checked for cross-browser compatibility, [including a check against IE11](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#cross-browser-compatibility)

### For maintainers

- [ ] This was checked for breaking API changes and was [labeled appropriately](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#release-notes-process)
spong pushed a commit to spong/kibana that referenced this pull request Mar 24, 2020
## Summary

Allow defining notifications that will trigger whenever the rule created new signals.

Requires:
- elastic#58395
- elastic#58964
- elastic#60832


![Screenshot 2020-03-02 at 10 19 18](https://user-images.githubusercontent.com/5188868/75662390-4fe8bf00-5c6f-11ea-943f-591367348b91.png)

![Screenshot 2020-03-02 at 10 13 00](https://user-images.githubusercontent.com/5188868/75662421-5e36db00-5c6f-11ea-9317-d158cddf4344.png)


### Checklist

Delete any items that are not applicable to this PR.

- [ ] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/master/packages/kbn-i18n/README.md)
- [ ] [Documentation](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#writing-documentation) was added for features that require explanation or tutorials
- [ ] [Unit or functional tests](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#cross-browser-compatibility) were updated or added to match the most common scenarios
- [ ] This was checked for [keyboard-only and screenreader accessibility](https://developer.mozilla.org/en-US/docs/Learn/Tools_and_testing/Cross_browser_testing/Accessibility#Accessibility_testing_checklist)
- [ ] This renders correctly on smaller devices using a responsive layout. (You can test this [in your browser](https://www.browserstack.com/guide/responsive-testing-on-local-server)
- [ ] This was checked for cross-browser compatibility, [including a check against IE11](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#cross-browser-compatibility)

### For maintainers

- [ ] This was checked for breaking API changes and was [labeled appropriately](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#release-notes-process)
spong added a commit that referenced this pull request Mar 25, 2020
## Summary

Allow defining notifications that will trigger whenever the rule created new signals.

Requires:
- #58395
- #58964
- #60832


![Screenshot 2020-03-02 at 10 19 18](https://user-images.githubusercontent.com/5188868/75662390-4fe8bf00-5c6f-11ea-943f-591367348b91.png)

![Screenshot 2020-03-02 at 10 13 00](https://user-images.githubusercontent.com/5188868/75662421-5e36db00-5c6f-11ea-9317-d158cddf4344.png)


### Checklist

Delete any items that are not applicable to this PR.

- [ ] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/master/packages/kbn-i18n/README.md)
- [ ] [Documentation](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#writing-documentation) was added for features that require explanation or tutorials
- [ ] [Unit or functional tests](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#cross-browser-compatibility) were updated or added to match the most common scenarios
- [ ] This was checked for [keyboard-only and screenreader accessibility](https://developer.mozilla.org/en-US/docs/Learn/Tools_and_testing/Cross_browser_testing/Accessibility#Accessibility_testing_checklist)
- [ ] This renders correctly on smaller devices using a responsive layout. (You can test this [in your browser](https://www.browserstack.com/guide/responsive-testing-on-local-server)
- [ ] This was checked for cross-browser compatibility, [including a check against IE11](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#cross-browser-compatibility)

### For maintainers

- [ ] This was checked for breaking API changes and was [labeled appropriately](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#release-notes-process)

Co-authored-by: patrykkopycinski <patryk.kopycinski@elastic.co>
@mikecote mikecote added release_note:enhancement and removed release_note:skip Skip the PR/issue when compiling release notes labels Apr 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Feature:Alerting release_note:enhancement Team:ResponseOps Platform ResponseOps team (formerly the Cases and Alerting teams) t// v7.7.0 v8.0.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[UI] Edit alert

6 participants